Ajustado services para classe de erros. - #449
Conversation
📝 WalkthroughWalkthroughThe change standardizes application exceptions across services and controllers. It adds structured external-service failures, domain-specific validation and authorization exceptions, centralized JSON or redirect rendering, updated feature tests, and an error-handling implementation plan. ChangesException handling alignment
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ProjectStageController
participant ProjectStageService
participant AppExceptionRenderer
Client->>ProjectStageController: advance stage
ProjectStageController->>ProjectStageService: advance
ProjectStageService-->>ProjectStageController: AppException
ProjectStageController-->>AppExceptionRenderer: propagate exception
AppExceptionRenderer-->>Client: JSON response or redirect with errors
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
# Conflicts: # app/Http/Controllers/ProjectStageController.php
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Exceptions/Integration/ExternalServiceException.php`:
- Around line 19-26: Update ExternalServiceException::fromFailedResponse so the
explicitly provided $service always remains authoritative when merging context
metadata. Replace the current spread-based merge with an array-union arrangement
that preserves ['service' => $service] even when $context contains a service
key.
In `@app/Http/Controllers/InstallmentController.php`:
- Line 41: Update the exception handling around SpreadsheetImporter::import() in
InstallmentController so InvalidArgumentException is caught before the broader
Throwable handler, preserving and returning its missing-header message instead
of replacing it with the generic error.
In `@app/Services/GoogleSheetsService.php`:
- Line 39: Update both Google Sheets fetch methods, including fetchSheet() and
the method at app/Services/GoogleSheetsService.php lines 219-219, to catch
ConnectionException alongside RequestException and wrap either in
ExternalServiceException; also add ExternalServiceException to fetchSheet()’s
declared exceptions.
In `@app/Services/MapasClient.php`:
- Around line 127-133: The MapasClient failure paths must stop storing
unsanitized upstream data in ExternalServiceException. In
app/Services/MapasClient.php lines 127-133, keep the error message stable using
only safe status and path fields, excluding the response body; at lines 210-214
and 228-229, redact query parameters from download URLs before storing or
reporting failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0fa00495-0c94-479c-b623-77d7a7de4445
📒 Files selected for processing (16)
app/Exceptions/Integration/ExternalServiceException.phpapp/Http/Controllers/InstallmentController.phpapp/Http/Controllers/ProjectController.phpapp/Http/Controllers/ProjectStageController.phpapp/Services/Documents/DocumentTypeRegistry.phpapp/Services/GoogleSheetsService.phpapp/Services/InstallmentImportService.phpapp/Services/MapasClient.phpapp/Services/NoticeService.phpapp/Services/ProjectDocumentService.phpapp/Services/ProjectStageService.phpapp/Services/ProjectSupervisorService.phpbootstrap/app.phptests/Feature/Document/DocumentTest.phptests/Feature/InstallmentImportServiceTest.phptests/Feature/ProjectStageFlowTest.php
…449) Registra no inventário/status da migração os ajustes feitos após o review: Import::validateRequiredColumns migrado para BusinessRuleException, e GoogleSheetsService/MapasClient refinados (captura de ConnectionException e sanitização de dados sensíveis nas mensagens de exceção). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/erros_plan.md (1)
41-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdicione a linguagem ao bloco Markdown.
O
markdownlintreporta MD040 na linha 41. Use```textou```treepara remover o aviso.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/erros_plan.md` at line 41, Atualize o bloco de código Markdown na seção correspondente de docs/erros_plan.md, adicionando um identificador de linguagem após as três crases de abertura, como text ou tree, para eliminar o aviso MD040 do markdownlint.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/erros_plan.md`:
- Around line 30-37: Atualize a descrição de ExternalServiceException no
documento para classificá-la como exceção de integração ou técnica, substituindo
a referência atual a exceção de domínio e mantendo consistente com a hierarquia
em app/Exceptions/Integration/.
- Around line 154-164: Alinhe o plano para usar uma única lista e contagem de
controllers: corrija o título e a tabela que enumeram os controllers com catch,
e remova ou ajuste as referências posteriores nas linhas 191 e 289 para não
incluir MonitoringController, NotificationController e PaymentController se
permanecerem fora do escopo. Preserve FormalizationController fora da migração
conforme indicado.
- Line 9: Atualize o plano em docs/erros_plan.md para distinguir claramente
etapas históricas já concluídas do trabalho ainda pendente: marque o rollout
como histórico ou remova etapas concluídas, incluindo a fundação e migrações
mencionadas nas seções próximas às linhas 184-195 e 282-291. Preserve apenas
ações realmente futuras e deixe explícito o estado atual do plano.
- Around line 110-122: Atualize a documentação do handler de renderização para
refletir o comportamento de bootstrap/app.php: use JSON apenas quando
expectsJson() e não houver X-Inertia; nos demais casos, retorne
back()->withErrors(...), removendo o fallback implícito. Corrija também as
referências nas seções associadas para documentar que
ProjectStageController::return() é uma exceção e preserva back()->with('error',
...), mantendo o fluxo esperado por ReturnProcessModal.vue.
---
Nitpick comments:
In `@docs/erros_plan.md`:
- Line 41: Atualize o bloco de código Markdown na seção correspondente de
docs/erros_plan.md, adicionando um identificador de linguagem após as três
crases de abertura, como text ou tree, para eliminar o aviso MD040 do
markdownlint.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec7bfce1-d808-4f40-936a-63872ca783cc
📒 Files selected for processing (5)
app/Exceptions/Integration/ExternalServiceException.phpapp/Services/GoogleSheetsService.phpapp/Services/MapasClient.phpapp/Support/Import.phpdocs/erros_plan.md
🚧 Files skipped from review as they are similar to previous changes (3)
- app/Exceptions/Integration/ExternalServiceException.php
- app/Services/MapasClient.php
- app/Services/GoogleSheetsService.php
…bbit
- Deixa claro que o doc virou registro histórico + checklist (partes já
implementadas, não mais uma proposta em aberto).
- Reclassifica ExternalServiceException como exceção técnica/integração
(estava descrita como "de domínio", contradizendo a hierarquia em
app/Exceptions/Integration/).
- Sincroniza o exemplo do handler de bootstrap/app.php com a implementação
real (ordem expectsJson/X-Inertia) e documenta a exceção do
ProjectStageController::return() que preserva back()->with('error', ...).
- Corrige contagem/lista de controllers (5, não 6) e remove
Monitoring/Notification/Payment da lista de trabalho restante, já que
não têm catch a migrar.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/erros_plan.md`:
- Line 287: Atualize o passo histórico 3 em docs/erros_plan.md para alinhar seu
escopo com a seção 3.1: marque as referências a SpreadsheetImportService e
PUMLGeneratorService como substituídas ou remova-as, mantendo o passo limitado a
GoogleSheetsService e MapasClient. Preserve a indicação de que os demais
controllers possuem o escopo descrito na linha sobre InstallmentController,
ProjectController, UserController e OpeningController.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 01c7ed56-05fb-4ca4-ba38-9de7bfaa9842
📒 Files selected for processing (1)
docs/erros_plan.md
…ação Passo 3 ainda citava SpreadsheetImportService e PUMLGeneratorService como alvo de ExternalServiceException, mas a seção 3.1 já os marca como fora de escopo. Restringe o passo a GoogleSheetsService e MapasClient. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/erros_plan.md`:
- Line 286: Before removing the legacy InvalidArgumentException renderer from
bootstrap/app.php, audit references across code, tests, and documentation
separately. Update the InvalidArgumentException throw in SyncProjectFilesJob and
preserve or revise the test asserting a 422 response, then verify no runtime
dependencies remain before deleting the handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 36d2dc88-3b15-4a1c-b492-d4bf3247c1da
📒 Files selected for processing (1)
docs/erros_plan.md
✅ Descrição do propósito desse Pull Request
🧭 Referência a Issue
❓ O que foi feito para atingir isso?
🏃♀️ Tipo de mudança
Marque as opções relevantes:
🕵️ Como foi testado?
Checklist: ✔️
Observação:
Summary by CodeRabbit
Bug Fixes
Documentation